fix(compliance): add /get_feature_flag endpoint to the test adapter#542
fix(compliance): add /get_feature_flag endpoint to the test adapter#542turnipdabeets wants to merge 10 commits into
Conversation
The harness was getting a 404 on /get_feature_flag, failing all 16 feature-flag tests. Implements the endpoint (mirrors posthog-ios): set person/group properties, reload flags, resolve the value, flush. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Satisfies the semgrep package-manager rules: add a 7-day cooldown to the Dependabot github-actions ecosystem and minimumReleaseAge: 10080 to the pnpm workspace, so newly published packages wait before being adopted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
posthog-android Compliance ReportDate: 2026-06-03 15:25:20 UTC
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 396ms |
| Format Validation.Event Has Uuid | ✅ | 36ms |
| Format Validation.Event Has Lib Properties | ✅ | 32ms |
| Format Validation.Distinct Id Is String | ✅ | 30ms |
| Format Validation.Token Is Present | ✅ | 30ms |
| Format Validation.Custom Properties Preserved | ✅ | 32ms |
| Format Validation.Event Has Timestamp | ✅ | 36ms |
| Retry Behavior.Retries On 503 | ✅ | 7031ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 4029ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 4025ms |
| Retry Behavior.Respects Retry After Header | ✅ | 7030ms |
| Retry Behavior.Implements Backoff | ✅ | 17035ms |
| Retry Behavior.Retries On 500 | ✅ | 7020ms |
| Retry Behavior.Retries On 502 | ✅ | 7019ms |
| Retry Behavior.Retries On 504 | ✅ | 7022ms |
| Retry Behavior.Max Retries Respected | ✅ | 17033ms |
| Deduplication.Generates Unique Uuids | ✅ | 36ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 7016ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 12030ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 7019ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 34ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 26ms |
| Compression.Sends Gzip When Enabled | ✅ | 19ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 17ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 13ms |
| Batch Format.Multiple Events Batched Together | ✅ | 31ms |
| Error Handling.Does Not Retry On 403 | ✅ | 4018ms |
| Error Handling.Does Not Retry On 413 | ❌ | 4014ms |
| Error Handling.Retries On 408 | ✅ | 5026ms |
Failures
error_handling.does_not_retry_on_413
Expected 1 requests, got 2
Feature_Flags Tests
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ❌ | 34ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 21ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 22ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 21ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 21ms |
| Request Payload.Groups Round Trip | ✅ | 22ms |
| Request Payload.Groups Default To Empty Object | ❌ | 27ms |
| Request Payload.Person Properties Distinct Id Auto Populated When Caller Omits It | ❌ | 25ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ❌ | 37ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 25ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ❌ | 27ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 10ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 26ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 2026ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 19ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 22ms |
Failures
request_payload.request_with_person_properties_device_id
Expected distinct_id='test_user_123', got '019e8e16-c5d7-70c7-a414-52ed4cae5c27'
request_payload.groups_default_to_empty_object
Field 'groups' not found in /flags request body at path 'groups'. Available keys: ['$anon_distinct_id', '$device_id', 'api_key', 'distinct_id', 'timezone', 'person_properties']
request_payload.person_properties_distinct_id_auto_populated_when_caller_omits_it
Field 'distinct_id' not found in /flags request body at path 'person_properties.distinct_id'. Available keys: ['$lib', '$lib_version', 'email']
request_payload.disable_geoip_false_propagates_as_geoip_disable_false
Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['$anon_distinct_id', '$device_id', 'api_key', 'distinct_id', 'timezone', 'person_properties']
request_payload.disable_geoip_omitted_defaults_to_false
Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['$anon_distinct_id', '$device_id', 'api_key', 'distinct_id', 'timezone', 'person_properties']
request_payload.flag_keys_to_evaluate_contains_only_requested_key
Field 'flag_keys_to_evaluate' not found in /flags request body at path 'flag_keys_to_evaluate'. Available keys: ['$anon_distinct_id', '$device_id', 'api_key', 'distinct_id', 'timezone', 'person_properties']
…n init Declare capabilities ["capture_v0", "encoding_gzip"] in /health so the harness runs the capture suites (android posts events to /batch and gzips them). Also set remoteConfig = false in /init so tests don't depend on the remote config load. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
sdk_compliance_adapter/src/main/kotlin/com/posthog/compliance/ComplianceAdapter.kt:352-354
**`ph.group()` fires an extra `$groupidentify` event and may trigger a second `/flags` request**
`PostHog.group()` calls `groupStateless()` internally, which calls `captureStateless(GROUP_IDENTIFY, ...)`, queuing a `$groupidentify` event. More critically, because `PostHog.reloadFeatureFlags: Boolean = true` by default, it also calls `reloadFeatureFlags(config?.onFeatureFlags)` whenever the group key is new. Since `/reset` clears persisted groups between tests, every test run sees a fresh group — making `reloadFeatureFlagsIfNewGroup` true every time. This means two `/flags` requests get made (one from `group()`, one from the explicit `reloadFeatureFlags { latch.countDown() }` below), and one `$groupidentify` batch request is created before the latch is even started. If the harness asserts exact batch or `/flags` request counts for group flag tests, those tests will remain red despite this fix.
Reviews (1): Last reviewed commit: "feat(compliance): declare capture capabi..." | Re-trigger Greptile |
|
|
||
| // Flush that event and wait so it can't spill into the next test's mock window. | ||
| ph.flush() | ||
| Thread.sleep(2000) |
There was a problem hiding this comment.
do we need this? Is there a better approach, like using a latch, waiting for the flush, or something?
Sleep slows down tests
There was a problem hiding this comment.
agreed, but tried this on iOS and it regressed 6 tests.
There was a problem hiding this comment.
either tests or the mock server would have to change to make this possible
most if not all of this should be gone IMO https://github.com/search?q=repo%3APostHog%2Fposthog-android+Thread.sleep&type=code
can be a follow up ofc
once we start adding sleeps here and there, its hard to stop, take a step back, and make it in a way that we dont need it anymore
There was a problem hiding this comment.
updated, no more sleep
|
bsed on #542 (comment) that didnt work? |
Use setGroupPropertiesForFlags(type, props, reloadFeatureFlags = false) for group_properties (no extra reload), matching the iOS adapter pattern. group() is still used for the type->key registration since the /flags `groups` field reads from the GROUPS preference, which only group() writes — its $groupidentify event and reload-on-new-group remain (no suppression param). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r-get-feature-flag # Conflicts: # pnpm-workspace.yaml
bfcf6e8 to
0f59b4e
Compare
SDK.reset() reloads flags as anon (PostHog.kt:1519 — intended behavior for
real-app logout), but that /flags lands in the next test's mock window and
inflates flag counts ("Expected 0, got 1" on no_flags_request_on_init_alone
and friends). close() has no network I/O; the next /init builds a fresh
instance. Matches the iOS adapter's pattern.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
e0fa4d5 to
639d0cd
Compare
Per review: the remoteConfig=false in /init claimed to stop a /flags reload, but PostHogRemoteConfig.kt:243 only chains to /flags when preloadFeatureFlags is true — so the config does nothing observable here. Dropping it. Also trims the capabilities and group() comments to one line each. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…pter - pass distinct_id through via identify() so /flags carries the caller's id - wipe the file-backed queue in /init and /reset so events don't replay across tests - surface (not swallow) a feature-flag reload timeout - document disable_geoip/force_remote as known SDK gaps - extract FLUSH_SETTLE_MS / QUEUE_STORAGE_PREFIX consts, import concurrency types Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
For some reason I can't see this comment |
its the 'posthog-android Compliance Report' where all tests were still not passing, so your changes didnt have any effect |
…d.sleep flush() is fire-and-forget with no SDK completion callback, so /flush and /get_feature_flag slept a fixed FLUSH_SETTLE_MS to let the batch reach the mock. Replace the blind sleep with a Condition signalled by the tracking interceptor once the /batch response is in hand (the mock has already recorded the request by then): returns the instant the batch lands, capped at FLUSH_SETTLE_MS so a never-sent flush can't hang. Addresses Manoel's review: deterministic wait, no sleeps. Safer than the fixed sleep since we only return after the HTTP round-trip is observed, never earlier. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
0220ed3 to
e1e4d6d
Compare
…a /flags identify() is the only public setter for the distinctId carried in /flags, but it unconditionally fires its own reload, adding a second /flags request per call. That regressed 7 harness tests that assert exact /flags counts (flags_request_*, groups_round_trip, two_flag_calls_produce_two_remote_requests, mock_response_value_is_returned_to_caller, get_feature_flag_captures_*). Revert to the deterministic single explicit reload. distinct_id is left decoded-but-unapplied (same as disable_geoip/force_remote): it cannot be set without forcing a second reload, so request_with_person_properties_device_id stays a known public-API gap rather than breaking 7 other tests. Verified locally against posthog-sdk-test-harness: 38/45, matching baseline with no regression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TL;DR: the compliance adapter was missing
/get_feature_flag→ harness 404s → 0/16 flag tests. This adds the endpoint (mirrors posthog-ios).